Fix /roll command silently doing nothing when called without arguments by Copilot · Pull Request #314 · Dans-Plugins/Medieval-Roleplay-Engine

Copilot · 2026-04-25T16:27:51Z

Fix /roll command: add default d20 roll, improve error handling, return true on success
Add description and usage to roll/dice commands in plugin.yml
Include rp.default in permission error message
Return true (not false) after sending custom error messages so Bukkit doesn't double-print the usage line
Remove hard-coded /roll [max] from the invalid-number error (works correctly for both /roll and /dice)
Fix CI build failure: add org.jetbrains:annotations:23.0.0 (provided) to pom.xml

…rn true on success Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Roleplay-Engine/sessions/a18471e3-ba4b-4758-b249-ceebd2267aaa Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot

Pull request overview

Updates the /roll (and /dice) command handling so it no longer silently does nothing when called without arguments, improving user feedback and default behavior in the MedievalRoleplayEngine command layer.

Changes:

Default /roll (and /dice) to a d20 when no max argument is provided.
Add numeric validation and explicit NumberFormatException handling with player-facing errors.
Add description and usage entries for roll and dice in plugin.yml.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`src/main/java/dansplugins/rpsystem/commands/roll/RollCommand.java`	Adds default die size, validates max argument, and improves error handling/return values for `/roll` & `/dice`.
`src/main/resources/plugin.yml`	Adds command metadata (`description`, `usage`) for `/roll` and `/dice`.

Copilot · 2026-04-25T16:59:40Z

+        Player player = (Player) sender;
+
+        if (!(player.hasPermission("rp.roll") || player.hasPermission("rp.dice") || player.hasPermission("rp.default"))) {
+            player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "Sorry! In order to use this command, you need one of the following permissions: 'rp.roll', 'rp.dice'");


The permission error message lists only 'rp.roll' and 'rp.dice', but this method also allows 'rp.default'. Please update the message to include 'rp.default' (or remove that permission from the check) so the feedback matches the actual access logic.

Suggested change

player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "Sorry! In order to use this command, you need one of the following permissions: 'rp.roll', 'rp.dice'");

player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "Sorry! In order to use this command, you need one of the following permissions: 'rp.roll', 'rp.dice', 'rp.default'");

Copilot · 2026-04-25T16:59:41Z

+
+        if (!(player.hasPermission("rp.roll") || player.hasPermission("rp.dice") || player.hasPermission("rp.default"))) {
+            player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "Sorry! In order to use this command, you need one of the following permissions: 'rp.roll', 'rp.dice'");
+            return false;


With usage now defined in plugin.yml, returning false here will cause Bukkit to also print the usage message after your custom error text, resulting in duplicate/extra output. Consider returning true after sending the permission-denied message (since the command was handled), and reserve false only for cases where you actually want Bukkit's usage line to be printed.

Suggested change

return false;

return true;

Copilot · 2026-04-25T16:59:41Z

+                if (max < 1) {
+                    player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "Please provide a positive number to roll.");
+                    return false;
                }


With usage now defined in plugin.yml, returning false here will make Bukkit print the usage line in addition to this custom message. If you want to show only your own validation message, return true after sending it (or remove the custom message and rely on Bukkit usage instead).

Copilot · 2026-04-25T16:59:41Z

+                    return false;
                }
+            } catch (NumberFormatException e) {
+                player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "'" + args[0] + "' is not a valid number. Usage: /roll [max]");


The error message hard-codes /roll [max], but this handler is also used for /dice (see CommandService). This will show the wrong usage when the player runs /dice .... Also, since plugin.yml now defines usage, returning false will cause Bukkit to print usage as well, duplicating the usage info you embed here. Consider either (a) not embedding the usage in the message and returning false, or (b) passing the command label into this method so you can format the correct usage and return true.

Suggested change

player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "'" + args[0] + "' is not a valid number. Usage: /roll [max]");

player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "'" + args[0] + "' is not a valid number.");

dmccoystephenson · 2026-04-25T18:07:57Z

@copilot You are a coding agent tasked with resolving all open comments and review threads
on this pull request. Work through every comment systematically and apply the
appropriate code changes.

Instructions

1. Discover all comments

Read every review thread, inline comment, and general PR comment
Include comments from: Copilot, human reviewers, and any other bots
Do not skip comments marked as suggestions — apply them if they make sense

2. For each comment, do the following:

Understand what is being requested or flagged
Locate the relevant file(s) and line(s)
Apply the fix directly in code
If a Copilot suggestion diff is provided, apply it (or an equivalent fix)
If the comment is ambiguous, use your best judgment and reply in the PR thread or review comment explaining what you did; only add a code comment when explicitly requested or when future maintainers would benefit from it
After addressing a comment, reply to the thread summarising what you did and mark it as resolved if your environment supports it

3. Types of issues to handle:

Code quality / style issues (naming, formatting, readability)
Logic bugs or edge cases flagged by reviewers
Security concerns (e.g. input validation, secrets, injection risks)
Performance suggestions
Missing or incomplete error handling
Test coverage gaps
Documentation / comment improvements
Dependency or import issues

4. Do NOT:

Dismiss or ignore any comment without acting on it — note that "acting" can also mean replying to the thread with a rationale (e.g., won't fix, already addressed elsewhere) and leaving the code unchanged when appropriate
Leave TODO comments as a substitute for fixing the issue
Break existing tests while addressing comments
Change functionality beyond the scope of what the comment asks for

After making changes

Ensure all existing tests still pass
Add or update tests if a comment explicitly asks for them, or if a bug fix warrants it
Double-check that every open thread has been addressed, replied to, and resolved

Context

This PR may contain Copilot-generated review comments — treat these with the same priority as human comments
If two comments conflict with each other, prefer the human reviewer's intent and document the conflict and resolution in the PR conversation or commit message, not in a code comment
Commit changes in logical groups if there are many unrelated fixes

…andled cases Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Roleplay-Engine/sessions/fd5e1444-ccf9-4a0e-baec-666c47cf5636 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot · 2026-04-25T18:08:59Z

@copilot You are a coding agent tasked with resolving all open comments and review threads
on this pull request. Work through every comment systematically and apply the
appropriate code changes....

Addressed all four review threads in commit 50d41bf:

Line 26: Added 'rp.default' to the permission error message so it matches the actual check.
Line 27: Changed return false → return true after the permission-denied message to prevent Bukkit from printing a duplicate usage line.
Lines 34–37: Changed return false → return true after the "positive number" validation error for the same reason.
Line 39: Removed the hard-coded /roll [max] from the invalid-number error message (it would show the wrong command for /dice users), and changed return false → return true to avoid duplicate usage output.

dmccoystephenson · 2026-04-26T00:06:21Z

@copilot fix failing CI check:

package org.jetbrains.annotations does not exist

Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Roleplay-Engine/sessions/39a4b3ee-48d6-46cd-b182-9dbf1379caa9 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot · 2026-04-26T00:08:10Z

@copilot fix failing CI check:

package org.jetbrains.annotations does not exist

Fixed in commit 580825c. Added org.jetbrains:annotations:23.0.0 as a provided dependency in pom.xml — PlaceholderAPI.java already used @NotNull from that package but it was never declared in the POM.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

hub.spigotmc.org
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.14/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.14/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.14 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.14/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/Medieval-Roleplay-Engine/Medieval-Roleplay-Engine org.codehaus.plexus.classworlds.launcher.Launcher compile -q (dns block)
jitpack.io
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.14/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.14/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.14 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.14/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/Medieval-Roleplay-Engine/Medieval-Roleplay-Engine org.codehaus.plexus.classworlds.launcher.Launcher compile -q (dns block)
repo.extendedclip.com
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.14/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.14/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.14 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.14/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/Medieval-Roleplay-Engine/Medieval-Roleplay-Engine org.codehaus.plexus.classworlds.launcher.Launcher compile -q (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Initial plan

2d156ef

Copilot AI assigned Copilot and dmccoystephenson Apr 25, 2026

Copilot started work on behalf of dmccoystephenson April 25, 2026 16:27 View session

Copilot AI linked an issue Apr 25, 2026 that may be closed by this pull request

rolling #313

Closed

Copilot AI changed the title ~~[WIP] Fix roll command not working in version 1.21.11~~ Fix /roll command silently doing nothing when called without arguments Apr 25, 2026

Copilot finished work on behalf of dmccoystephenson April 25, 2026 16:33

Copilot AI requested a review from dmccoystephenson April 25, 2026 16:33

dmccoystephenson requested a review from Copilot April 25, 2026 16:56

Copilot started reviewing on behalf of dmccoystephenson April 25, 2026 16:57 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Copilot started work on behalf of dmccoystephenson April 25, 2026 18:08 View session

Copilot finished work on behalf of dmccoystephenson April 25, 2026 18:09

dmccoystephenson marked this pull request as ready for review April 26, 2026 00:03

Copilot started work on behalf of dmccoystephenson April 26, 2026 00:06 View session

Fix CI: add org.jetbrains:annotations dependency to pom.xml

580825c

Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Roleplay-Engine/sessions/39a4b3ee-48d6-46cd-b182-9dbf1379caa9 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot finished work on behalf of dmccoystephenson April 26, 2026 00:08

dmccoystephenson approved these changes Apr 26, 2026

View reviewed changes

dmccoystephenson merged commit b38b095 into main Apr 26, 2026
1 check passed

dmccoystephenson deleted the copilot/fix-roll-command-issue branch April 26, 2026 00:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix /roll command silently doing nothing when called without arguments#314

Fix /roll command silently doing nothing when called without arguments#314
dmccoystephenson merged 4 commits intomainfrom
copilot/fix-roll-command-issue

Copilot AI commented Apr 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

dmccoystephenson commented Apr 25, 2026

Uh oh!

Copilot AI commented Apr 25, 2026

Uh oh!

dmccoystephenson commented Apr 26, 2026

Uh oh!

Copilot AI commented Apr 26, 2026 •

edited

Loading

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "Sorry! In order to use this command, you need one of the following permissions: 'rp.roll', 'rp.dice'");
	player.sendMessage(medievalRoleplayEngine.colorChecker.getNegativeAlertColor() + "Sorry! In order to use this command, you need one of the following permissions: 'rp.roll', 'rp.dice', 'rp.default'");

Conversation

Copilot AI commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

dmccoystephenson commented Apr 25, 2026

Instructions

After making changes

Context

Uh oh!

Copilot AI commented Apr 25, 2026

Uh oh!

dmccoystephenson commented Apr 26, 2026

Uh oh!

Copilot AI commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Apr 25, 2026 •

edited

Loading

Copilot AI commented Apr 26, 2026 •

edited

Loading